This seems good to me. To summarise:

1. If you don't select any target, it tries them all and "skips" targets that aren't deemed appropriate.
2. If you explicitly select an appropriate target, it succeeds.
3. If you explicitly select an inappropriate target, it fails.

In scenario (1), I can see in the screenshot two things that would be nice to improve:

• The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.
• Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

I don't mind deferring that to another PR though. WDYT @czechboy0 @MahdiBM?

@simonjbeaumont
I know this PR has been going for long enough that you probably don't want to bother me anymore, but those changes are small enough, and even if they weren't, i still wouldn't mind them 😅

The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.

Good idea but will basically duplicate the logs because we'll need to say running on target '----' then run failed on target '-----'. (Unless the target is just straight up not a swift source module in which case no need for duplication.)

Should i go for it? or did you mean something else?

Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

No objections, but can you mention some examples so i know what you exactly have in mind?
Like just changing the words, or kind of flagging the print statements by starting them with e.g. Skipped target '----':, Warnings for target '-----':?
EDIT: Ah reading your message again, you probably just meant changing the word. Though i still like the other idea 🤔

What i do prefer though, is for the Docs part to be deferred to another PR considering this has taken enough time/length 😅 would prefer this to be merged sooner than later, when possible, so i can also start using the main Apple repo in my Package.swifts 🙂

@simonjbeaumont I know this PR has been going for long enough that you probably don't want to bother me anymore, but those changes are small enough, and even if they weren't, i still wouldn't mind them 😅

OK, great!

Both of these comments pertain to this logging from your screenshot above, so I'll paste the textual version here before continuing to discuss your specific questions:

✅ Invoking plug-in command "OpenAPIGeneratorCommand" 18.5 seconds
- Target 'PennyTests': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Models': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Penny': OpenAPI code generation failed. No expected config or OpenAPI document files.
Swift OpenAPI Generator is running with the following configuration:
- OpenAPI document path: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/openapi.yaml
- Configuration path: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/openapi-generator-config.yml
- Generator modes: types, client
- Output file names: Types.swift, Client.swift
- Output directory: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/GeneratedSources
- Diagnostics output path: <none - logs to stderr>
- Current directory: /Users/mahdibm/Github/penny-bot
- Plugin source: command
- Additional imports: <none>
File Types.swift: unchanged
File Client.swift: unchanged
- Target 'GitHubAPI': OpenAPI code generation successfully completed.
- Target 'Extensions': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'GHHooksLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Fake': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'SponsorsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'SharedServices': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'CoinsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'FaqsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'AutoPingsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
✅ Activity Log Complete 7/20/23, 10:50 AM 17.8 seconds
No issues

The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.

Good idea but will basically duplicate the logs because we'll need to say running on target '----' then run failed on target '-----'. (Unless the target is just straight up not a swift source module in which case no need for duplication.)

Should i go for it? or did you mean something else?

Personally, I prefer logging to be before an operation, and, looking through the build logs in Xcode for a target I happen to have open I see this pattern. Examples include Compiling ..., and Discovering Swift tasks after .... When an operation is failed, there is some "duplicate" logging; here is a textual version of what I see in the report navigator when I replace print with pint (note, the emojis aren't actually in the log—they are a representation of how Xcode renders the icons):

...
✅ Planning Swift module swift_openapi_generator (arm64)
ℹ️ Compiling runGenerator.swift 
    ℹ️ Compile runGenerator.swift (arm64)
        ❌ Cannot find 'pint' in scope
❌ Build failed ...

When the issue if fixed, I see:

✅ Planning Swift module swift_openapi_generator (arm64)
✅ Compiling runGenerator.swift 
    ✅ Compile runGenerator.swift (arm64)
...
✅ Build succeeded ...

Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

No objections, but can you mention some examples so i know what you exactly have in mind? Like just changing the words, or kind of flagging the print statement by starting them with e.g. Skipped target '----':, Warnings for target '-----':?

I think it's odd to see big green check marks and lots of <some operation> failed in the logs.

I'm not suggesting we go for emojis, but I'd be happy with us finding some terminology that is clear about what's going on, for example:

Considering target for OpenAPI generation: InappropriateTarget
Skipped target (no OpenAPI document): InappropriateTarget
Considering target for OpenAPI generation: AppropriateTarget
Running Swift OpenAPI Generator on target AppropriateTarget with the following configuration:
...

I'd be happy if it then printed a line explicitly saying whether it was succeeded or failed.

IMHO this small amount of "duplicate" logging is a small price to pay for clarity when debugging a failed build.

@simonjbeaumont This is what i came up with.

Please let me know if i can improve any of the sentences.

I used the ✅ emoji for a successful code generation because i think it makes long logs like this more skimmable.

The - Stopping because target isn't configured for OpenAPI code generation. logs don't feel satisfying in terms of being different from the theme/mood of the rest of the logs, but that's the best i could come up with.

@simonjbeaumont This is what i came up with.
Thanks for going the extra mile on this one.

I used the ✅ emoji for a successful code generation because i think it makes long logs like this more skimmable.

While I initially hinted at not doing this, I think I agree; it does make it easier to read at a glance so I'm fine with it 👍

Please let me know if i can improve any of the sentences.

The - Stopping because target isn't configured for OpenAPI code generation. logs don't feel satisfying in terms of being different from the theme/mood of the rest of the logs, but that's the best i could come up with.

Aside from tone, this one in particular isn't informative—why isn't it "configured" and what does it mean for it to be configured? I this case we could have something like: - Skipped target: Target does not contain OpenAPI document or something similar?

Other than that I'm pleased with how this is looking.

@czechboy0 do you have anything to add before we push this over the line?

@simonjbeaumont i made it non-informative because i suspected @czechboy0 would not like the duplication of "there are no files" messages as he requested some changes about them in the past reviews.

Though i think both views are completely valid. It's just a question of if the information is worth the duplication. I'm personally fine with both, maybe slightly leaning towards the current non-informative message.

@simonjbeaumont i made it non-informative because i suspected @czechboy0 would not like the duplication of "there are no files" messages as he requested some changes about them in the past reviews.

Though i think both views are completely valid. It's just a question of if the information is worth the duplication. I'm personally fine with both, maybe slightly leaning towards the current non-informative message.

OK, that's fair enough, and sorry for missing that historical review context.

I'm happy with things the way they are now (subject to the soundness check passing again).

Honestly i had no idea what some of the warnings were requesting:

** ✅ Found no unacceptable language.
** Running /code/scripts/check-license-headers.sh...
** ERROR: Unsupported file extension for file (exclude or update this script): Plugins/OpenAPIGenerator/PluginsShared
** Running /code/scripts/run-swift-format.sh...
Plugins/PluginsShared/PluginUtils.swift:4:92: warning: [UseEarlyExits] replace the `if/else` block with a `guard` statement containing the early exit
Plugins/PluginsShared/PluginUtils.swift:4:92: warning: [UseEarlyExits] replace the `if/else` block with a `guard` statement containing the early exit
/code/Package.swift:173:10: warning: [TrailingComma] add trailing comma to the last element in multiline collection literal
Package.swift:164:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:14:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:21:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:97:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:99:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:104:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:106:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginUtils.swift:4:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:58:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:59:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:62:1: warning: [LineLength] line is too long
** ERROR: ❌ Running swift-format produced errors.

  To fix, run the following command:

    % git ls-files -z '*.swift' | xargs -0 swift-format --in-place --parallel

A few of the warnings were also pointing at non-existent (or wrong) lines (like Package.swift being 172 lines and swift-format complaining about the 173 rd line!)
But luckily i noticed the CI also mentions git ls-files -z '*.swift' | xargs -0 swift-format --in-place --parallel which automatically solved the problems.

EDIT: Not to mention the line is too long warnings which were in short complaining about something that couldn't have been solved, and even the command didn't really fix anything, it just moved the long lines to the line after the returns which is the best it could do.

@simonjbeaumont @czechboy0 what should i do to resolve this last soundness error?

** ✅ Found no unacceptable language.
** Running /code/scripts/check-license-headers.sh...
** ERROR: Unsupported file extension for file (exclude or update this script): Plugins/OpenAPIGenerator/PluginsShared
** Running /code/scripts/run-swift-format.sh...
** ✅ Ran swift-format with no errors.

Had to exclude 2 to directories from check-license-headers.sh.
These are the symlinked directories, and the actual files are still checked for headers, as it threw errors for the files that didn't have the headers in the second to last CI run.

read -ra PATHS_TO_CHECK_FOR_LICENSE <<< "$( \
  git -C "${REPO_ROOT}" ls-files -z \
  ":(exclude).gitignore" \
  ":(exclude).spi.yml" \
  ":(exclude).swift-format" \
  ":(exclude).github/*" \
  ":(exclude)CODE_OF_CONDUCT.md" \
  ":(exclude)CONTRIBUTING.md" \
  ":(exclude)CONTRIBUTORS.txt" \
  ":(exclude)LICENSE.txt" \
  ":(exclude)NOTICE.txt" \
  ":(exclude)Package.swift" \
  ":(exclude)Package.resolved" \
  ":(exclude)README.md" \
  ":(exclude)SECURITY.md" \
  ":(exclude)scripts/unacceptable-language.txt" \
  ":(exclude)Tests/PetstoreConsumerTests/Generated" \
  ":(exclude)Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/*" \
  ":(exclude)docker/*" \
  ":(exclude)**/*.docc/*" \
  ":(exclude)**/.gitignore" \
  ":(exclude)**/Package.swift" \
  ":(exclude)**/Package.resolved" \
  ":(exclude)**/README.md" \
  ":(exclude)**/openapi.yaml" \
  ":(exclude)**/openapi.yml" \
  ":(exclude)**/petstore.yaml" \
  ":(exclude)**/openapi-generator-config.yaml" \
  ":(exclude)**/openapi-generator-config.yml" \
+  ":(exclude)Plugins/OpenAPIGenerator/PluginsShared" \
+  ":(exclude)Plugins/OpenAPIGeneratorCommand/PluginsShared" \
  | xargs -0 \
)"

Thanks @MahdiBM for bearing with us during this marathon of a PR 🙂 Just updated from main and will land once CI passes.